Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dependencies: Add support for Python 3.12 #131

Merged
merged 4 commits into from
Dec 2, 2024
Merged

Conversation

sphuber
Copy link
Collaborator

@sphuber sphuber commented Oct 26, 2023

No description provided.

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.35%. Comparing base (61cb138) to head (e326b9a).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #131      +/-   ##
==========================================
+ Coverage   90.32%   90.35%   +0.04%     
==========================================
  Files          15       15              
  Lines        1125     1129       +4     
==========================================
+ Hits         1016     1020       +4     
  Misses        109      109              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@unkcpz
Copy link
Member

unkcpz commented Nov 27, 2024

@sphuber, can you also add the fixes for #138? I think we can just pinning the version of aio-pika to ~=9.4.0?

@sphuber
Copy link
Collaborator Author

sphuber commented Dec 1, 2024

Docs failing with

WARNING: failed to reach any of the inventories with the following issues:
intersphinx inventory 'https://aio-pika.readthedocs.io/en/latest/objects.inv' not fetchable due to <class 'requests.exceptions.HTTPError'>: 404 Client Error: Not Found for url: https://aio-pika.readthedocs.io/en/latest/objects.inv

@sphuber
Copy link
Collaborator Author

sphuber commented Dec 1, 2024

@unkcpz applied the aio-pika pin but unfortunately there are still two tests failing that don't seem trivial to fix.

@unkcpz
Copy link
Member

unkcpz commented Dec 1, 2024

Right, I'll look into it.

EDIT: the doc build is fixed by updating the aio-pika doc link.

@unkcpz
Copy link
Member

unkcpz commented Dec 1, 2024

I think the failed test came from a070bfe
In _outcom_destroyed the coroutine might be called from the other thread therefore it requires the run_coroutine_threadsafe. Why you made this change in the first place @sphuber?

This eager_task_factory feature of py3.12 seems more like call_soon_threadsafe.
In the outcome destroy case it seems the correct async primitive to use, but I think it is better for another PR to check. @sphuber can you revert that commit?

@unkcpz
Copy link
Member

unkcpz commented Dec 1, 2024

Thanks for revert the change @sphuber. Now there are two tests failed and I'll try to look into it.

@unkcpz
Copy link
Member

unkcpz commented Dec 1, 2024

I didn't find anything in https://docs.python.org/3/whatsnew/3.12.html#asyncio that may cause the failed test after using python3.12. But my guess is that performance improvement was by tightly schedule the task into even loop and run without another round of event loop between schedule requeue and task_queue.get. The change in the #139 is explicitly relinquish the control before get task from queue to make sure all the requeue coroutines are scheduled.

Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems all works!

unkcpz and others added 3 commits December 2, 2024 08:59
This allows relinquishing control to the event loop to ensure requeueing
coroutines have the chance to get run before another task is fetched
from the queue.
Latest release 9.5.0 breaks due to an import error of `typing_extensions`.
See mosquito/aio-pika#649
@sphuber sphuber merged commit 18248d4 into master Dec 2, 2024
17 checks passed
@sphuber
Copy link
Collaborator Author

sphuber commented Dec 2, 2024

Fantastic, thanks a lot for the help @unkcpz

@sphuber sphuber deleted the feature/python-3.12 branch December 2, 2024 08:05
@sphuber sphuber mentioned this pull request Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants